Add view_component integration for Datadog::Tracing
#4977
+839
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* `ViewComponent` supports instrumentation through `ActiveSupport` notifications, and having the ability to trace a ViewComponent using Datadog's APM provides a similar level of debugging ease that tracing `ActionView` template and partial rendering provides * This tracing component was based on `Datadog::Tracing::Contrib::ActionView`, with some key differences: * There is only one action available for instrumentation: `render` * ViewComponent is technically its own component, not part of Rails itself * Therefore, it uses `Contrib::Integration#auto_instrument?` rather than checking the Railtie * The `span.resource` is the name of the component (`MyComponent`), not its identifier (`my_component.rb`), for a better user experience * Added tests, but need to update the Matrixfile and appraisals
y9v
reviewed
Oct 17, 2025
There was a problem hiding this comment.
@tcannonfodder thank you for your contribution!
I left some small remarks, and I think it would make sense to add an integration test for this feature. If you need some help with the integration test, please let me know.
| end | ||
|
|
||
| option :service_name | ||
| option :component_base_path do |o| |
There was a problem hiding this comment.
Would components_base_path be better? Since it's a path containing multiple components.
There was a problem hiding this comment.
I also think we would need to ensure the correct format here - when calculating the component identifier, we are relying that this path will end with /, otherwise component identifiers will start with a /
There was a problem hiding this comment.
Both this (and the comment below) are essentially straight ports from the action_view integration; which felt like the right approach to take here because these components are run together (you often render ViewComponents inside ActionView), so it feels right that they are configured and behave similarly.
But! Y’all have the final say here; so just lemme know what makes sense for the repo. :) I just wanted to explain my reasoning for these decisions.
| sections_view = identifier.split(base_path) | ||
|
|
||
| if sections_view.length == 1 | ||
| identifier.split('/')[-1] |
There was a problem hiding this comment.
I think this won't work if the components base path is repeated somewhere in the filename:
> 'some/path/components/admin/components/sidebar_component.rb'.split('components/')[-1]
=> "sidebar_component.rb"
Thanks so much for reviewing it! Eager to get this merged in 😄 . I left some additional contextual comments for why I’d made those decisions; just let me know which approaches you’d like me to take. I’ll try writing an integration test; and I’ll make a new comment with a WIP commit if I get stuck. |
Strech
approved these changes
Oct 21, 2025
| end | ||
|
|
||
| def subscriptions | ||
| all.collect(&:subscriptions).collect(&:to_a).flatten |
There was a problem hiding this comment.
I think it could be a flat_map with a single iteration.
y9v
approved these changes
Oct 21, 2025
|
I haven't forgotten the integration test; just got swamped this weekend! 😵💫 Will get that done this week 💪 |
|
@Strech, @y9v I figured out what was happening here (configuration changes in ViewComponent + needing the right incantations to prepare a Rails test app for the test suite). I am keeping the old comment for posterity; but the integration test has been written. Once we're good with it, I can make the other code changes suggested in the comments! Outdated comment belowI tried my best to get an integration test prepped & ready, but ran into configuration/buildchain issues at the last mile. See the following commits: In short: if I try running the test task, I get the following error: However, running the individual command has the tests pass: Thanks so much for your help! |
* Older versions of ViewComponent had a different notification name for ActiveSupport notifications, `!render.view_component` * See: ViewComponent/view_component#1771 * To support this, there is a configuration flag, `:use_deprecated_instrumentation_name`, which mirrors the name of the configuration option within `ViewComponent` itself * If set to true, it changes the `event_name` for the `ViewComponent::Events::Render#event_name` to be `!render.view_component`
* I was not able to get the `docker compose` setup running on my machine due to the inability to pull the `ddapm-test-agent` * Therefore, I was not able to spool up containers for each version of Ruby to generate their `gemfiles/` * While I tested this locally with `ruby-3.3`, I did not commit the changes since I was not able to follow the containerized setup
* Followed the documentation to add `DD_TRACE_VIEW_COMPONENT_ENABLED` to `supported-configurations.json`, then run the task to generate `lib/datadog/core/configuration/supported_configurations.rb`
* In order to write working integration tests for the major versions of ViewComponent that have instrumentation support, we need to use custom appraisal sets instead of `build_coverage_matrix`, since an ad-hoc Rails app needs to be spooled up, which includes `pg`, `rails`, and `sprockets < 4` for the max versions of Rails for `ViewComponent` `2.34.0 * I was not able to get the `docker compose` setup running on my machine due to the inability to pull the `ddapm-test-agent` * Therefore, I was not able to spool up containers for each version of Ruby to generate their `gemfiles/` * While I tested this locally with `ruby-3.3`, I did not commit the changes since I was not able to follow the containerized setup
* In order to properly write an integration test for ViewComponent we need to spin up an ad-hoc Rails app that we render the component within, then confirm the span data is generated * The integration test includes the `datadog/tracing/contrib/rails/rails_helper`, which provides a `Rails test application` context * Since there were configuration changes between different versions of ViewComponent, we need to adjust the configuration settings based on which version of the Gem has been loaded * This can be done using `Gem.loaded_specs` and `Gem::Version`, which provide an API for version comparison * Testing across the Appraised versions also checks that the `use_deprecated_instrumentation_name` configuration is applied, and does collect the necessary data
tcannonfodder
force-pushed
the
tcannonfodder/add-view_component-tracing
branch
from
a4ba6e0 to
1e2b4c7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Add
view_componentintegration forDatadog::TracingViewComponentsupports instrumentation throughActiveSupportnotifications, and having the ability to trace a ViewComponent using Datadog's APM provides a similar level of debugging ease that tracingActionViewtemplate and partial rendering providesDatadog::Tracing::Contrib::ActionView, with some key differences:renderContrib::Integration#auto_instrument?rather than checking the Railtiespan.resourceis the name of the component (MyComponent), not its identifier (my_component.rb), for a better user experienceWIP: Add
view_componentbuild matricies in appraisaldocker composesetup running on my machine due to the inability to pull theddapm-test-agentgemfiles/ruby-3.3, I did not commit the changes since I was not able to follow the containerized setupMotivation:
I'm using
ViewComponentin my apps, and wanted to be able to debug performance issues inside of a ViewComponent.Change log entry
Additional Notes:
Example screenshot:
How to test the change?
Tests have been added based on the
action_viewintegration